Skip to content

refactor: expand all abbreviated identifiers to full words#800

Merged
tcoratger merged 2 commits into
leanEthereum:mainfrom
tcoratger:refactor/expand-abbreviations-upstream
May 29, 2026
Merged

refactor: expand all abbreviated identifiers to full words#800
tcoratger merged 2 commits into
leanEthereum:mainfrom
tcoratger:refactor/expand-abbreviations-upstream

Conversation

@tcoratger
Copy link
Copy Markdown
Collaborator

Motivation

A reference specification should read as explicitly as possible. Abbreviated identifiers (att_data, validator_id, msg, sk, …) make the spec ambiguous. This PR spells every abbreviated identifier out in full across src/, tests/, and the packages/ testing framework — ~280 distinct renames — and codifies the convention as a rule in CLAUDE.md.

What changed

Python identifiers only (variables, parameters, functions, methods, classes, attributes, constants) — never on-the-wire strings.

  • validator_idvalidator_index (the domain-correct term)
  • attattestation, msgmessage, sigsignature, sksecret_key, pk/pubkeypublic_key
  • idxindex, prevprevious, currcurrent, aggaggregate, propproposal, connconnection, privkeyprivate_key, lenlength
  • Function/method/class names too: split_by_msgsplit_by_message, PubkeyPublicKey, get_attestation_pubkeyget_attestation_public_key
  • MSG_LEN_FEMESSAGE_LENGTH_FIELD_ELEMENTS

Deliberately kept (verified case-by-case)

  • Canonical protocol IDs: peer_id, node_id, protocol_id, subnet_id, stream_id
  • ENR fields attnets / seq, the num_* prefix, the reqresp libp2p name
  • Library/stdlib APIs: argparse dest=, model_config, tmp_path, CERT_NONE, …
  • External symbols, e.g. functions imported from lean_multisig_py (split_type_2_by_msg)

Format-affecting notes

Internal config/fixture string keys were updated to stay consistent with the renamed fields. Test vectors regenerate, but cross-client tooling may reference these:

  • Genesis YAML keys: attestation_pubkey/proposal_pubkeyattestation_public_key/proposal_public_key
  • Validator registry manifest keys (*_pubkey_hex, *_privkey_file*_public_key_hex, *_private_key_file)
  • One metrics coverage-section label: "agg_start_new""aggregate_start_new"

Verification

  • just check passes (ruff, format, ty type-check, codespell, mdformat)
  • Full unit suite passes: 3084 passed

🤖 Generated with Claude Code

A reference specification should read as explicitly as possible, so every
abbreviated identifier is spelled out in full across src, tests, and the
packages/ testing framework (~280 distinct renames).

Highlights:
- validator_id -> validator_index (the domain-correct term)
- att -> attestation, msg -> message, sig -> signature, sk -> secret_key,
  pk/pubkey -> public_key, idx -> index, prev -> previous, agg -> aggregate,
  prop -> proposal, conn -> connection, privkey -> private_key, len -> length
- function/method/class names too (split_by_msg -> split_by_message,
  Pubkey -> PublicKey, get_attestation_pubkey -> get_attestation_public_key)
- MSG_LEN_FE -> MESSAGE_LENGTH_FIELD_ELEMENTS

Kept verbatim: canonical protocol IDs (peer_id, node_id, protocol_id,
subnet_id, stream_id), ENR fields (attnets, seq), num_* prefixes, the
reqresp protocol name, library APIs (argparse dest, model_config, tmp_path),
and external symbols (e.g. lean_multisig_py functions like split_type_2_by_msg).

Internal config/fixture string keys were updated to match the renamed
fields (genesis YAML keys, validator registry manifest keys, one metrics
coverage-section label). Test assertions/fixtures referencing identifiers
via strings (parametrize names) were aligned.

Adds a NO ABBREVIATIONS IN IDENTIFIERS rule to CLAUDE.md documenting the
convention and its canonical exceptions.

just check passes (ruff, format, ty, codespell, mdformat).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@tcoratger tcoratger force-pushed the refactor/expand-abbreviations-upstream branch from c99ba5d to b6d63a0 Compare May 29, 2026 17:08
The attribute self._keys_dir was renamed to self._keys_directory, but the
matching __slots__ string entry must be renamed too -- otherwise setting the
attribute raises AttributeError (slotted classes have no __dict__).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@tcoratger tcoratger merged commit 034ad0c into leanEthereum:main May 29, 2026
13 checks passed
tcoratger added a commit to unnawut/leanSpec that referenced this pull request May 30, 2026
…renames

Rebase onto main and apply the renames/cleanups landed since the branch
was opened:

- PR leanEthereum#799: TypeOneMultiSignature → SingleMessageAggregate, proof_type
  literal "type_1" → "single_message", file renames
  test_type_1_{valid,invalid}.py → test_single_message_{valid,invalid}.py.
- PR leanEthereum#800: validator_ids → validator_indices, with_validator_id →
  with_validator_index, vid/pubkey expansions.
- post-leanEthereum#788/leanEthereum#790/leanEthereum#796 imports: lean_spec.spec.forks / .spec.ssz /
  .spec.crypto.*.

Replace the stringly-typed tamper dict with a Pydantic discriminated
union (RebindToAlternateHeadRoot, IncrementEmittedSlot,
SwapParticipantPublicKey). The match dispatch on the typed variants
drops the two type: ignore[index] casts and lets the test sites read
tamper=SwapParticipantPublicKey(index=0, with_validator_index=1)
instead of a magic-string dict.

Inline the four single-call helpers (_apply_tamper plus three
_tamper_*) and the verification helper into make_fixture so the four
phases — generate / tamper / verify / publish — are visible in one
method.

Drop both model_copy(update=...) calls per leanEthereum#789: direct field
construction for the frozen AttestationData rebuild, direct field
assignment for the mutable fixture self-update.

Replace the internal dict[str, Any] bundle with named locals; the
bundle never escapes make_fixture, so the dict adds nothing.

Guard SwapParticipantPublicKey against silent no-op swaps where the
replacement key happens to equal the original. Broaden the verifier
exception catch to surface unexpected exception types as
"expected X got Y" instead of crashing the filler.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
tcoratger added a commit that referenced this pull request May 30, 2026
…heck proof verification (#786)

* test(consensus): add VerifyProofsTest fixture and Type-1 valid vectors

Introduces a new consensus fixture format that emits self-contained
multi-signature verification vectors so cross-client implementations
can run their own Type-1 verifier and compare outcomes.

Three positive vectors land alongside the fixture: a single-validator
baseline, a four-validator all-participating case, and a
four-validator non-contiguous bitfield case ([1, 0, 1, 1]).

The fixture also surfaces the spec-layer binding between attestation
data and proof: clients recompute hash_tree_root(attestation_data)
and must match the emitted message field before running the verifier.

* test(consensus): add Type-1 verify_proofs rejection vectors

Adds four negative vectors exercising spec-layer bindings between
inputs and the multi-signature proof. Each vector uses a tamper
operation on the fixture to produce a structurally valid bundle that
must be rejected by a conformant verifier:

- wrong_message: proof bound to an alternate head root inside the
  attestation data
- wrong_slot: emitted slot field shifted while the proof binding
  stays on the original slot
- wrong_public_keys: one emitted pubkey replaced with another
  validator's
- aggregation_bits_length_mismatch: emitted bits truncated while
  the pubkey count stays unchanged

Vectors covering malformed or truncated proof bytes are intentionally
out of scope: leanSpec consumes the multi-signature primitive as a
black box and primitive integrity belongs to its own conformance
suite. Pubkey ordering is also not a binding to test: the aggregator
sorts participants internally, so the verifier is order-insensitive.

* test(consensus): align VerifyProofsTest with sibling fixture conventions

Brings the new fixture in line with the patterns the other consensus
test fixtures follow:

- Drop ``from __future__ import annotations`` (PR #759 removed it from
  Pydantic-defining files); quote the one self-reference instead.
- Replace the bespoke ``expect_valid: bool`` field with the inherited
  ``expect_exception`` field already used by SSZTest, NetworkingCodec,
  and VerifySignaturesTest. Rejection vectors now pin
  ``AggregationError`` and the framework serializes the class name to
  JSON.
- Switch the tamper dispatch in ``_apply_tamper`` from ``if/elif`` to
  ``match/case`` to follow the pattern in slot_clock and
  networking_codec.
- Expand the module-level docstring from one line to a short
  paragraph describing what the fixture covers.
- Normalize the ``public_keys`` default from ``[]`` to ``| None =
  None`` to match every other output field on the model.

* test(consensus): drop aggregation_bits_length_mismatch rejection vector

The check that fires here is the early-reject in the spec wrapper's
verify method (len(public_keys) != participants.count(True)), not a
consensus-critical binding. In real consensus the inconsistency cannot
arise because clients resolve public keys from the bitfield plus the
validator registry as one operation. A client that did pass a wrong
pubkey count would also be rejected by the underlying recursive
verifier on its internal pubkey-set commitment, so the wrapper check
is at best an early exit with a nicer error message.

The remaining three rejection vectors still exercise the meaningful
spec-layer bindings: message hash, slot, and pubkey set.

* refactor(consensus): tighten VerifyProofsTest and absorb post-rebase renames

Rebase onto main and apply the renames/cleanups landed since the branch
was opened:

- PR #799: TypeOneMultiSignature → SingleMessageAggregate, proof_type
  literal "type_1" → "single_message", file renames
  test_type_1_{valid,invalid}.py → test_single_message_{valid,invalid}.py.
- PR #800: validator_ids → validator_indices, with_validator_id →
  with_validator_index, vid/pubkey expansions.
- post-#788/#790/#796 imports: lean_spec.spec.forks / .spec.ssz /
  .spec.crypto.*.

Replace the stringly-typed tamper dict with a Pydantic discriminated
union (RebindToAlternateHeadRoot, IncrementEmittedSlot,
SwapParticipantPublicKey). The match dispatch on the typed variants
drops the two type: ignore[index] casts and lets the test sites read
tamper=SwapParticipantPublicKey(index=0, with_validator_index=1)
instead of a magic-string dict.

Inline the four single-call helpers (_apply_tamper plus three
_tamper_*) and the verification helper into make_fixture so the four
phases — generate / tamper / verify / publish — are visible in one
method.

Drop both model_copy(update=...) calls per #789: direct field
construction for the frozen AttestationData rebuild, direct field
assignment for the mutable fixture self-update.

Replace the internal dict[str, Any] bundle with named locals; the
bundle never escapes make_fixture, so the dict adds nothing.

Guard SwapParticipantPublicKey against silent no-op swaps where the
replacement key happens to equal the original. Broaden the verifier
exception catch to surface unexpected exception types as
"expected X got Y" instead of crashing the filler.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

---------

Co-authored-by: Thomas Coratger <60488569+tcoratger@users.noreply.github.com>
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant